I wonder why you didn't use EntityComponentsTrigger for MutateEvent like the other lifecycle observers do?

I'm 50/50 on this, but mainly its because a Mutation shouldn't ever be an structural change, so no archetypes will ever change, so passing them is assumed to be useless information. (There is also overhead for Mutate to pass them in the queue path)
The components inside the Event instead of the Trigger, is more of a personal preference, but we already store the target Entity inside the Event, and so having all the data of which to match an Observer, be on the Event seemed fitting.

a Mutation shouldn't ever be an structural change, so no archetypes will ever change, so passing them is assumed to be useless information.

Mhm yeah passing archetypes here is not ideal either. Edit: could we maybe move them onto the various lifecycle events as ArchetypeId?

The components inside the Event instead of the Trigger, is more of a personal preference, but we already store the target Entity inside the Event, and so having all the data of which to match an Observer, be on the Event seemed fitting.

Why don't we do this for the other lifecycle events too then?

Storing the components on the Event also forces you to use Vec instead of a borrowed slice since Event must be 'static.

Mhm yeah passing archetypes here is not ideal either. Edit: could we maybe move them onto the various lifecycle events as ArchetypeId?

I have thought and suggested a thing before, we would need to discuss with Doot as they are working on a Replace Event and are wanting direct access to Archetypes for perf, but perhaps it would still be okay with ArchetypeId's?

Why don't we do this for the other lifecycle events too then?

Storing the components on the Event also forces you to use Vec instead of a borrowed slice since Event must be 'static.

Same as above, I would prefer it, but some Birbs have a differing opinion and so we would need to discuss that.

Having to create a Vec<ComponentId> for every MutateEvent feels pretty wasteful.

I thought about it, and its kinda wasteful regardless, either way most Lifecycle events are creating temporary slices and passing that around which creates copies of the whole slice on the stack, where this just copies the Vec pointer to the heap. It also makes triggering the Event significantly easier, since there is no longer any lifetimes involved, which is also makes it ergonomic/easy to use with Commands.

way most Lifecycle events are creating temporary slices and passing that around which creates copies of the whole slice on the stack, where this just copies the Vec pointer to the heap

I'm not sure what you mean here, the slice is passed as part of the trigger, which is always passed as a reference, just like the event is, and thus the slice is never copied onto the stack. But eitherway this cost is significantly lower than creating a whole new heap allocation. I encourage you to benchmark this, but I suspect no benchmark is gonna have good results with a heap allocation in this critical path.

which is also makes it ergonomic/easy to use with Commands

I'm not sure why someone would want to manually trigger a MutateEvent through commands. This should only be triggered by the ECS.

I'm not sure what you mean here, the slice is passed as part of the trigger, which is always passed as a reference, just like the event is, and thus the slice is never copied onto the stack. But eitherway this cost is significantly lower than creating a whole new heap allocation. I encourage you to benchmark this, but I suspect no benchmark is gonna have good results with a heap allocation in this critical path.

This is only because the 'normal' path for current Lifecycle Events are all done at sync-points via an Entity Command, so they are all guaranteed the apply path which makes references always valid, but if needed in a queue path then it requires you to copy the Id's anyways, and relookup Archetype references.

I'm not sure why someone would want to manually trigger a MutateEvent through commands. This should only be triggered by the ECS.

Allowing manual access is always a necessity, otherwise it takes away the 'engine code is user code' feel. There are no invariants required by Mutate to not be manually triggered, so it would be an artificial limitation of that fact you are passing references instead of Id's.
I've tried working on Lifecycle Events for Disabled Component, and it was alot more difficult then it needed to be since I was forced to use Events via Commands, and trying to use Lifecycle Events with Commands is as stated above, is less then ideal.

This SAFETY comment doesn't really explain what's going on here.

Yeah I am not exactly sure what to put for most of these, except that we have Deferred World Access, which implies we cant change anything structurally, and most of the unsafe is only unsafe because of that.
Like I could explain that whole sentence out, but it shows up in like 8 places in the single function all for the same reasons.

except that we have Deferred World Access, which implies we cant change anything structurally,

This is completely unrelated to this unsafe usage though?

You're calling trigger_on_mutate, whose safety doc says "Caller must ensure ComponentId in target exist in self". Here you should then explain why it's guaranteed that the ComponentIds in comps.iter().copied() all exist in Self.

It seems to me that this is not guaranteed because this function takes a Vec<ComponentId> as input and is safe, so the caller can pass in any ComponentId, even those that don't exist in this World.

The trigger_raw call down below also has a safety requirement that's completely unrelated to the use of DeferredWorld, and the other unsafe usages are on a &mut World but still mention DeferredWorld-Access!

I can go through and update the Safety comments for the trigger_raw and trigger_on_mutate to match their Safety comments on the method.
The other unsafes are all done via DeferredWorld::deref()->&World::* there is no &mut World access in any Mutate function, so that invariant is what is upholding those, again I could explain why only having DeferredWorld access implies that but that is already covered by the DeferredWorld type itself.

I got confused by the self.commands().queue(move |world: &mut World| { (which does give you a &mut World), but I failed to notice that you later convert it to a DeferredWorld. Given this then the branches are again almost the same:

if APPLY {
    // ...
} else {
    self.commands().queue(move |world: &mut World| {
        // SAFETY: We have exclusive access to [`World`] in [`Command`]
        let mut world = unsafe { world.as_unsafe_world_cell().into_deferred() };
        // ... same as before
    });        
}

Btw you can safely get a DeferredWorld from a &mut World using DeferredWorld::from(world), no need to use unsafe there.

This looks very hard to read 😅

I tried to split things up as much as possible and condense the code where I could, but most of what is in this methods is unique to the impl.
Unless you meant the lack of comments I suppose, which is fair.

It appears to me that there's quite some duplication in this code though. For example the two parts that do a.get_storage_type(*c).is_some_and(|s| { ... } look completely equal.

Your right, I overlooked the Storage match and scan, that could be abstracted out.

Actually when looking it over, there are three versions with slight differences:

1. Multiple Components, Multiple Entities (Query)
2. Single Resource, Single Entity (ResMut)
3. Single Resource, Multiple Entity (FilteredResourcesMut)

Both 1 and 3 are the most alike, but slightly different in that 1 needs to return and collect the Components to trigger them as a single Event for the Entity, but 3 does not need to do that and can instead trigger inline because it is a Resource and they are not shared between entities.

2 is similar but does more upfront, since it knows its dealing with only a single value/entity.

So while they technically could be merged/abstracted I think it would just lead to either confusing code or worse performance.

It seems that mutate hooks are not called when e.g. EntityMut::get_mut is used to mutate components.

Do you mean from an exclusive System / &mut World? I didnt impl for World as a SystemParam, we could do that.
The others were straight forward and had little options for alternatives, but direct world access is probably best done with a direct push style, or a new Mut wrapper that has access to Commands to trigger an Mutate on DerefMut.

I mean even outside a system.

If I create a new world, register a Mutate event/hook, then mutate a component using EntityMut::get_mut, then I would expect the event/hook to fire.

Yeah so probably a custom Mut wrapper, or adding to the existing one returned by World.

I've got an branch stacked on this one, that has a working doc-test for this:

    /// #[derive(Component, Debug)]
    /// #[component(on_mutate)]
    /// pub struct Comp(pub u32);
    ///
    /// impl Comp {
    ///     fn on_mutate(mut world: DeferredWorld<'_>, hook: HookContext) {
    ///         let c = world.entity(hook.entity).get::<Comp>().unwrap();
    ///         assert!(false, "Hook: {c:?}");
    ///     }
    /// }
    ///
    /// fn observer(on: On<Mutate<Comp>>, query: Query<&Comp>) {
    ///     let c = query.get(on.entity).unwrap();
    ///     assert!(false, "Observer: {c:?}");
    /// }
    ///
    /// let mut world = World::default();
    /// world.add_observer(observer);
    /// {
    ///     let mut e = world.spawn(Comp(0));
    ///     let mut c = e.into_mut_c::<Comp>().unwrap();
    ///     c.0 = 25;
    /// }
    /// world.flush(); // Asserts with "Hook: Comp(25)", or "Observer: Comp(25)" if no hook.

Have to go through and replace all the instances of Mut from direct World-access now.
For the impl, I made another Wrapper, that Wraps Mut and an EntityDeferredWorld. (EntityDeferredWorld = (Entity, DeferredWorld))

I think this PR really needs some benchmarks to understand the impact of this feature.

Yeah, I've never written any and the setup for it looks a bit daunting 😅
I do agree though, I would be interested in seeing some numbers, because when I ran the bevy_city example with and without this PR, I used an entity observer for each car with and without Transform as an match term, and couldn't notice anything given that my FPS was already pretty inconsistent.